Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(builder): put the upper limit on reallocation #1748

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

NamanJain8
Copy link
Contributor

@NamanJain8 NamanJain8 commented Sep 23, 2021

z.Allocator imposes an upper bound of 1GB on the size of the allocator. In builder, we double the allocation while reallocating to amortize the cost over the future allocations.

Consider a scenario where the size of KV is 700MB. The size of the allocator would be 700MB. Now we want to finish the block, so we will need some bytes for the metadata. That can easily fit into the remaining 324MB. But we were earlier doing over-allocation that causes panic.

panic: Unable to allocate more than 1073741824


goroutine 306 [running]:
github.com/dgraph-io/ristretto/z.(*Allocator).Allocate(0xc002898800, 0x5780034a, 0x6, 0x0, 0xa1)
	/home/algod/go/pkg/mod/github.com/dgraph-io/[email protected]/z/allocator.go:285 +0x2ec
github.com/dgraph-io/badger/v3/table.(*Builder).allocate(0xc00015c360, 0x4, 0x7f0a01dc8768, 0xc002658bb8, 0x0)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/table/builder.go:108 +0xc5
github.com/dgraph-io/badger/v3/table.(*Builder).append(0xc00015c360, 0xc002578008, 0x4, 0x4)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/table/builder.go:118 +0x39
github.com/dgraph-io/badger/v3/table.(*Builder).finishBlock(0xc00015c360)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/table/builder.go:275 +0x57
github.com/dgraph-io/badger/v3/table.(*Builder).Done(0xc00015c360, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/table/builder.go:428 +0x6e
github.com/dgraph-io/badger/v3/table.CreateTable(0xc000042288, 0x12, 0xc00015c360, 0xc000042288, 0x12, 0x6565656565656565)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/table/table.go:259 +0x45
github.com/dgraph-io/badger/v3.(*sortedWriter).createTable(0xc002832e70, 0xc00015c360, 0x0, 0x0)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/stream_writer.go:545 +0x690
github.com/dgraph-io/badger/v3.(*sortedWriter).send.func1(0xc002832e70, 0xc00015c360)
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/stream_writer.go:500 +0x35
created by github.com/dgraph-io/badger/v3.(*sortedWriter).send
	/home/algod/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/stream_writer.go:499 +0x89


This change is Reviewable

@NamanJain8 NamanJain8 merged commit f762055 into master Sep 23, 2021
@NamanJain8 NamanJain8 deleted the naman/allocator-limit branch September 23, 2021 08:14
NamanJain8 added a commit that referenced this pull request Oct 5, 2021
z.Allocator imposes an upper bound of 1GB on the size of the allocator. In builder, we double the allocation while reallocating to amortize the cost over the future allocations.

Consider a scenario where the size of KV is 700MB. The size of the allocator would be 700MB. Now we want to finish the block, so we will need some bytes for the metadata. That can easily fit into the remaining 324MB. But we were earlier doing over-allocation that causes panic.

(cherry picked from commit f762055)
mangalaman93 pushed a commit that referenced this pull request Feb 14, 2023
z.Allocator imposes an upper bound of 1GB on the size of the allocator. In builder, we double the allocation while reallocating to amortize the cost over the future allocations.

Consider a scenario where the size of KV is 700MB. The size of the allocator would be 700MB. Now we want to finish the block, so we will need some bytes for the metadata. That can easily fit into the remaining 324MB. But we were earlier doing over-allocation that causes panic.

(cherry picked from commit f762055)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants